-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
HHH-18974 - initialize SecureRandom at runtime #9657
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ide the UuidVersion7Strategy constructor Signed-off-by: Jan Schatteman <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is to rather remove Holder and adding a private final SecureRandom numberGenerator = new SecureRandom(); to UuidVersion7Strategy, which will require that State#getNextState accepts a SecureRandom parameter.
I've already talked with Yoann about a potential solution; the PR is currently only partial. |
…ategy.Holder classes Signed-off-by: Jan Schatteman <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM assuming you managed to get tests to pass on Quarkus (as we discussed), thanks!
The idea is to rather remove
Holderand adding aprivate final SecureRandom numberGenerator = new SecureRandom();toUuidVersion7Strategy
Unfortunately if we do that, the UuidVersion7Strategy#INSTANCE, along with its SecureRandom, will end up in the heap just before native compilation, which will lead to the exact same problem.
Your suggestion could potentially work if the numberGenerator in UuidVersion7Strategy was lazily-initialized -- but even then I'm not entirely sure.
The removal of the numberGenerator access is indeed not enough, it's just the first part of a two-fold solution:
- First commit: making sure that no access is made to
numbergeneratorbefore we actually generate UUIDs (i.e., at runtime for native applications). - Second commit: making sure that any native application using Hibernate ORM will initialize the Holder classes at runtime, ensuring the
SecureRandom(and its seed) will be initialized at runtime.
This is enough to fix the issue in theory, and Jan checked this by running integration tests in my ORM 7 upgrade PR, so we should be fine.
hibernate-graalvm/src/test/java/org/hibernate/graalvm/internal/StaticClassListsTest.java
Outdated
Show resolved
Hide resolved
69a5ea2 to
1e013d6
Compare
hibernate-core/src/main/java/org/hibernate/id/uuid/UuidVersion6Strategy.java
Show resolved
Hide resolved
…imeInitialization() Signed-off-by: Jan Schatteman <[email protected]>
Signed-off-by: Jan Schatteman <[email protected]>
1e013d6 to
9b22d27
Compare
inside the UuidVersion7Strategy constructor
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.
https://hibernate.atlassian.net/browse/HHH-18974